Spike/base site approach a - #21853
Conversation
Resolve SSR baseSiteId in plain Node, no Angular. Cacheless by design: every resolve() fetches OCC base sites fresh and matches the request URL against each site's urlPatterns, falling back to the app-configured default (context.baseSite[0]) on no match. No cache because SSR runs as multiple instances that die/restart independently, so a per-process cache would drift between nodes and yield inconsistent results across the fleet; the OCC call is cheap, so consistency beats the saved call. Reliability (spec factor #1 - never hang/DoS the SSR process): - concurrency cap (default 10) sheds load fast, throwing ConcurrencyLimitError before touching OCC; - AbortController timeout (default 3000ms) throws OccUnavailableError. Framework provides createBaseSiteRequestHandler(): it resolves the baseSiteId from a trust-proxy-aware request URL and maps both typed errors to 503 + Retry-After. The app wires only the route and a render callback; the framework carries no HTTP or site knowledge.
- use protected instead of private for resolver fields and methods - rename misleading _config constructor param to config - apply contentType to the 503 error response in the request handler - drop stale "switch approaches" note from the resolver contract
🚨 PR Title Validation Failed 🚨Your pull request title does not follow the required format. Please update it to match the expected pattern: Expected format: Allowed Types
Example of a valid PR title✅ ❌ Merge is blocked until the PR title is corrected. |
CX_BASE_URL is not a Node runtime env var on CCv2; read OCC base URL from build-time environment.occBaseUrl first, then occ-backend-base-url meta tag in browser/index.csr.html (substituted by the deploy script). Missing URL degrades gracefully — warning logged, AI-SEO handlers disabled, core SSR unaffected (previously crashed on startup). Resolver gains 60 s TTL cache with initPromise dedup to avoid thundering-herd on cache miss; concurrency cap guards the miss path.
🚨 PR Title Validation Failed 🚨Your pull request title does not follow the required format. Please update it to match the expected pattern: Expected format: Allowed Types
Example of a valid PR title✅ ❌ Merge is blocked until the PR title is corrected. |
There was a problem hiding this comment.
for all these files, since the concern is base-site and not site-context at large, I recommend the next directory to be renamed to base-site instead of site-context
| err instanceof ConcurrencyLimitError || | ||
| err instanceof OccUnavailableError |
There was a problem hiding this comment.
minor comment: since this is a spike
We are defaulting to 5 second retry for both concurrency limit error and occ unavailable.
I don't they they should share the retry value.
for ConcurrencyLimitError, I would suggest 2-3 seconds
for OccUnavailableError, I would suggest 20-30 seconds
| protected readonly timeoutMs: number; | ||
| protected readonly maxConcurrentOccCalls: number; | ||
| protected readonly cacheTtlMs: number; | ||
| protected readonly defaultBaseSite: string | null; |
There was a problem hiding this comment.
I have a small issue with naming this defaultBaseSite. Not because of the name itself (without additional context this name is correct). Why my issue? Because the existing base site configuration uses defaultBaseSite already as a const for the list of base sites. Perhaps with this spike, since we are already refactoring base site files, we rename the array to baseSites and keep this one as is
No description provided.